Skip to content
This repository has been archived by the owner on Jun 11, 2023. It is now read-only.

Enable Steps to be restarted #383

Merged
merged 3 commits into from
Feb 11, 2018

Conversation

renatomassaro
Copy link
Member

@renatomassaro renatomassaro commented Feb 10, 2018

Closes #333. Closes #274.

Now you can delete your cracker and still proceed with the tutorial! 🎉

TODO:

  • Internal refactor for reuse of Steppable.setup (made it idempotent)
  • StepRestartedEvent
  • Rollback emails / messages on checkpoint defined at StepRestartedEvent
  • Docs (marked as DOCME)
  • Specs missing at SetupStory
  • Update Story.Step meta when data has been regenerated

Notes

  • Unlike Steppable.start, Steppable.restart seems generic (it does not change for each step) and as such can be abstracted into a single function, so I think there's no need to implement it on each Step declaration.
    Due to a tiny detail (checkpoint (email) metadata is defined on the restart method), this cannot be done. A refactor where we make the whole Step interface 100% declarative would solve this, but there's no time for that.

Incidental

  • Renamed Steppable.fail to Steppable.restart. Failure no longer exists 🙌.
  • Split old Steppable.setup/1 into two parts, Steppable.start/1 and Steppable.setup/1. Now the setup only does the setup, and is reused at Steppable.restart.
  • Steppable.setup/1 must be idempotent, so existing code was updated.
  • setup_once macro to quickly enable idempotent object search/creation at Steppable.setup
  • Added FileDeletedEvent. Proper handlers on FilesystemHandlers are missing.
  • StoryQuery.Setup for standardized idempotency search during Steppable.setup
  • Removed FK on story_emails

This change is Reviewable

@sourcelevel-bot
Copy link

Ebert has finished reviewing this Pull Request and has found:

  • 2 possible new issues (including those that may have been commented here).
  • 1 fixed issue! 🎉

You can see more details about this review at https://ebertapp.io/github/HackerExperience/Helix/pulls/383.

@renatomassaro
Copy link
Member Author

Reviewed 10 of 28 files at r1, 35 of 35 files at r2.
Review status: all files reviewed at latest revision, 8 unresolved discussions.


events.json, line 200 at r2 (raw file):

        },
        "DownloadCracker": {
          "filters": ["File.Downloaded"],

Also filters File.Deleted (btw File.Deleted and Step.Restarted must be added to the JSON file)


lib/software/event/handler/filesystem.ex, line 25 at r2 (raw file):

  # Existing entries being removed
  # TODO FileDeletedEvent

Create issue


lib/story/internal/email.ex, line 67 at r2 (raw file):

    do: generic_send(step, reply_id, :player)

  def rollback_email(step, email_id, meta) do

doc + spec


lib/story/model/step/macros.ex, line 320 at r2 (raw file):

  end

  defmacro setup_once(object, identifier, do: block),

docme


lib/story/model/story/email.ex, line 90 at r2 (raw file):

  end

  def rollback_email(entry, email_id, meta) do

doc + spec


lib/story/model/story/email.ex, line 99 at r2 (raw file):

  end

  defp do_rollback_email(changeset, email) do

defp should be placed at the end of the file


test/support/story/helper.ex, line 47 at r2 (raw file):

  end

  @doc """

Move these more important functions up


test/support/story/vars.ex, line 3 at r2 (raw file):

defmodule Helix.Test.Story.Vars do
  @moduledoc """
  This helper will inject (in a non-higienic way!)

Oops


Comments from Reviewable

@renatomassaro
Copy link
Member Author

0️⃣ 🆒


Reviewed 7 of 7 files at r3.
Review status: all files reviewed at latest revision, 8 unresolved discussions.


events.json, line 200 at r2 (raw file):

Previously, renatomassaro (Renato Massaro) wrote…

Also filters File.Deleted (btw File.Deleted and Step.Restarted must be added to the JSON file)

Done.


lib/software/event/handler/filesystem.ex, line 25 at r2 (raw file):

Previously, renatomassaro (Renato Massaro) wrote…

Create issue

Done.


lib/story/internal/email.ex, line 67 at r2 (raw file):

Previously, renatomassaro (Renato Massaro) wrote…

doc + spec

Done.


lib/story/model/step/macros.ex, line 320 at r2 (raw file):

Previously, renatomassaro (Renato Massaro) wrote…

docme

Done.


lib/story/model/story/email.ex, line 90 at r2 (raw file):

Previously, renatomassaro (Renato Massaro) wrote…

doc + spec

Done.


lib/story/model/story/email.ex, line 99 at r2 (raw file):

Previously, renatomassaro (Renato Massaro) wrote…

defp should be placed at the end of the file

Done.


test/support/story/helper.ex, line 47 at r2 (raw file):

Previously, renatomassaro (Renato Massaro) wrote…

Move these more important functions up

Done.


test/support/story/vars.ex, line 3 at r2 (raw file):

Previously, renatomassaro (Renato Massaro) wrote…

Oops

Done.


Comments from Reviewable

@renatomassaro renatomassaro merged commit aad5209 into HackerExperience:master Feb 11, 2018
@renatomassaro renatomassaro deleted the story-progress branch February 11, 2018 07:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant